-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented: logic to update notification icon after viewing and added notification preference card on the settings page (#301) #304
Conversation
…d preference card on the settings page (#301)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to abstract naming from Ofbiz to OMS generic
}, | ||
async updateNotificationPref(enumId: string, event: any) { | ||
try { | ||
const ofbizInstanceName = this.userProfile.ofbizInstanceName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be "omsInstanceName" we can't make this specific to Ofbiz @ravilodhi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, got it. Will make changes to the Get Use Profile API and the respective occurrences on the UI side. Currently, this variable does not hold an OMS instance name, it is going to be a unique keyword. Currently "hotwax" is getting set for all the instances during deployment and used internally for some operations.
I had a discussion with Deepak Sir to set this to some unique value (most probably the OMS instance) so that the firebase topic can be created uniquely per OMS instance.
@dt2patel
Related Issues
Closes #301
Short Description and Why It's Useful
Screenshots of Visual Changes before/after (If There Are Any)
Contribution and Currently Important Rules Acceptance